Skip to content

fix: add missing support for oneOf directive in schema printer #1727

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 9, 2025

Conversation

jasonbahl
Copy link
Contributor

@jasonbahl jasonbahl commented Jul 8, 2025

Fix: Add Missing Test and Support for @oneOf Directive in Schema Printer

Summary

This PR addresses missing test coverage and implementation gaps for the @oneOf directive in the schema printer. While PR #1715 implemented the core @oneOf directive support, it was missing tests to ensure the directive is properly printed when schemas are serialized, and the underlying build cycle was not preserving the directive.

This fix ensures full compatibility with the GraphQL.js reference implementation for schema printing and rebuilding.

Missing Test Coverage

  • Added testInputTypeWithOneOfDirective() in SchemaPrinterTest.php to verify @oneOf directive printing for input types with isOneOf: true
  • Added testCorrectlyExtendInputObjectTypeWithOneOfDirective() in BuildSchemaTest.php to cover @oneOf directive parsing from input object extensions
  • Matches GraphQL.js test pattern from printSchema-test.ts

Schema Printer Fix

  • Modified printInputObject() in SchemaPrinter.php to check $type->isOneOf() and append @oneOf directive when true
  • Preserves existing formatting and integrates seamlessly with current printer logic

Build Schema Fix

  • Enhanced makeInputObjectDef() in ASTDefinitionBuilder.php to:
    • Parse @oneOf directive from input object definition nodes using Values::getDirectiveValues()
    • Parse @oneOf directive from extension nodes (previously uncovered code path)
    • Set isOneOf property correctly when building InputObjectType instances
  • Added proper exception handling with @throws annotations for PHPStan compliance

Root Cause

The issue occurred because:

  1. SchemaPrinter::printInputObject() was not checking the isOneOf property
  2. ASTDefinitionBuilder::makeInputObjectDef() was not parsing the @oneOf directive from AST nodes
  3. This broke the build cycle: InputObjectTypeprintbuildprint was not preserving the directive

Test Coverage

  • Schema printing verification: Ensures @oneOf directive appears in printed schema
  • Build cycle verification: Confirms directive is preserved through SchemaPrinter::doPrint()BuildSchema::build()SchemaPrinter::doPrint() cycle
  • Extension scenario coverage: Verifies @oneOf directive parsing from both main definitions and extension nodes
  • Backward compatibility: All existing tests continue to pass

Breaking Changes

None. This is a bug fix that adds missing functionality without changing existing behavior.

Validation Results

  • ✅ All schema printer tests passing (49/49)
  • ✅ All build schema tests passing (66/66)
  • ✅ All utils tests passing (402/402)
  • ✅ PHPStan analysis clean (0 errors)
  • ✅ Complete build cycle preservation verified
  • ✅ Extension directive parsing verified

Example Scenarios

Main Definition:

input SearchInput @oneOf {
  byId: ID
  byName: String
}

Extension Definition:

input SearchInput {
  byId: ID
}

extend input SearchInput @oneOf {
  byName: String
}

Both scenarios now properly preserve the @oneOf directive through the complete build cycle.

@spawnia spawnia merged commit 0f35b73 into webonyx:master Jul 9, 2025
18 checks passed
@spawnia
Copy link
Collaborator

spawnia commented Jul 9, 2025

Thank you, released with https://github.com/webonyx/graphql-php/releases/tag/v15.21.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants